Ported TraktKit to tvOS #36

Michelasso · 2019-06-29T09:00:56Z

Converted frameworks to Universal frameworks
Added test for tvOS and macOS

I have also changed a couple of other things. For example I couldn't fetch the avatars, the "user" structure (If I remember correctly. I had actually changed it nearly a year ago and ported the change to this new fork). needed to be more nested.

Also I renamed "set" to "setOauth2RedirectURL", just deprecating the old "set". But I can revert it if you don't like the new name, it just made more sense to me.

Now with Universal Frameworks one just need to add

import TraktKit

no matter the OS type. Thanks to this now all tests are common for iOS, tvOS and macOS (same source files, I just added the targets) and most important, they all succeeded. Also the prototype app I was developing compiled and run without any issue (and using all my modifications).

This is more or less the same work I did for TheMovieDatabaseSwiftWrapper, which has been merged months ago.

Last thing: I had also modified "getUserWatchedHistory" adding the pagination, but when I ported my old function it gave an error compiling. Also I am a bit lost, because I see there is a "getHistory". Are they any different (I have checked the Trakt TV APIs and I must have missed "getHistory")? Should I make a new pull request (after this one) to add the pagination in "getUserWatchedHistory"?

Converted frameworks to Universal frameworks Added test for tvOS and macOS

Michelasso · 2019-06-30T10:25:07Z

Please wait to merge. I have a couple of issues coming from bad copy&paste. Also I am cleaning it a bit from my comments

Michelasso · 2019-07-01T05:35:16Z

Common/MLKeychain.swift

@@ -40,7 +40,7 @@ public class MLKeychain {
        let keychainQuery: [String: Any] = [
            kSecClassValue: kSecClassGenericPasswordValue,
            kSecAttrAccountValue: key,
-            kSecReturnDataValue: kCFBooleanTrue,
+            kSecReturnDataValue: kCFBooleanTrue!,


Not sure why I have been told by Xcode to fix this one. It was working a year ago I suppose?

I'll have to look into this some. I'd love to avoid any force unwraps.

…ktKitMacOS (removed TraktKitMacOS-tests)

Michelasso · 2019-07-01T10:01:39Z

Ok, it should be fine, now. I forgot to mention that I also added the methods "getDeviceCode" and "getTokenFromDeviceCode", useful to login with a code (to insert via a web browser after authenticating).

All tests are working, just giving a warning because I deprecate "set()" as explained above.

Also it has been "ported" by Xcode to Swift 5.0 (Xcode made no changes to the source code).

MaxHasADHD · 2020-03-31T12:01:08Z

Common/MLKeychain.swift

+            if #available(iOSApplicationExtension 11.3, *) {
+                #if DEBUG
+                print("[\(#function)] Security result error code is: \(String(SecCopyErrorMessageString(status, nil) ?? "UNKNOWN"))")
+                #endif
+            } else {
+                // Fallback on earlier versions
+            }


Thanks for this!

MaxHasADHD · 2020-03-31T12:03:43Z

Common/Wrapper/TraktManager.swift

+    public func setOauth2RedirectURL(withClientID: String, clientSecret secret: String, redirectURI: String, staging: Bool = false) {
+        self.clientID = withClientID
+        self.clientSecret = secret
+        self.redirectURI = redirectURI
+
+        self.staging = staging
+
+        self.baseURL = !staging ? "trakt.tv" : "staging.trakt.tv"
+        self.APIBaseURL = !staging ? "api.trakt.tv" : "api-staging.trakt.tv"
+        self.oauthURL = URL(string: "https://\(baseURL!)/oauth/authorize?response_type=code&client_id=\(withClientID)&redirect_uri=\(redirectURI)")
+    }


I think this should be reverted. set could use a better signature but I'm iffy on setOauth2RedirectURL, this can be a different PR / change to the tvOS support.

Sure, no problem. I just wanted to give it a different name because "set" was a bit too generic.

MaxHasADHD · 2020-03-31T12:05:01Z

Common/Wrapper/TraktManager.swift

+    public var deviceCode: String?
+    public var userCode: String?
+    public var verificationURL: String?
+    public var timeInterval: TimeInterval?
+    public var expiresIn: TimeInterval?


I'll have to look into the API some more before I know if a struct would be better than the variables kept in this class. I would recommend adding more documentation from the Trakt API to define all of these.

Yeah, I had looked into that. I wasn't sure either. But I needed them to make the authorization functions working on tvOS.

MaxHasADHD · 2020-03-31T12:05:18Z

Common/Models/Users/TraktUser.swift

@@ -16,13 +16,15 @@ public struct User: Codable {
    public let name: String?
    public let isVIP: Bool?
    public let isVIPEP: Bool?
+    //    public let ids: ID


These changes should be reverted, I just merged a PR that added support for this.

MaxHasADHD · 2020-03-31T12:05:35Z

Common/MLKeychain.swift

@@ -40,7 +40,7 @@ public class MLKeychain {
        let keychainQuery: [String: Any] = [
            kSecClassValue: kSecClassGenericPasswordValue,
            kSecAttrAccountValue: key,
-            kSecReturnDataValue: kCFBooleanTrue,
+            kSecReturnDataValue: kCFBooleanTrue!,


I'll have to look into this some. I'd love to avoid any force unwraps.

MaxHasADHD · 2020-03-31T12:06:17Z

Common/Wrapper/TraktManager.swift

+        }
+
+
+        let urlString = "https://\(baseURL!)/oauth/device/code"


Avoid force unwrapping.

MaxHasADHD · 2020-03-31T12:07:10Z

Common/Wrapper/TraktManager.swift

+                if let deviceCodeDict = try JSONSerialization.jsonObject(with: data, options: []) as? [String: AnyObject] {
+
+                    welf.deviceCode = deviceCodeDict["device_code"] as? String
+                    welf.userCode = deviceCodeDict["user_code"] as? String
+                    welf.verificationURL = deviceCodeDict["verification_url"] as? String
+                    welf.timeInterval = deviceCodeDict["interval"] as? TimeInterval
+                    welf.expiresIn = deviceCodeDict["expires_in"] as? TimeInterval


I think decoding into an object would be preferable to this old way of decoding JSON

MaxHasADHD · 2020-03-31T12:07:27Z

Common/Wrapper/TraktManager.swift

+                return
+        }
+
+        let urlString = "https://\(baseURL!)/oauth/device/token"


force unwrap

MaxHasADHD · 2020-03-31T12:08:05Z

TraktKitMacTests/TraktKitMacTests.swift

+    override func setUp() {
+        // Put setup code here. This method is called before the invocation of each test method in the class.
+
+        // In UI tests it is usually best to stop immediately when a failure occurs.
+        continueAfterFailure = false
+
+        // UI tests must launch the application that they test. Doing this in setup will make sure it happens for each test method.
+        XCUIApplication().launch()
+
+        // In UI tests it’s important to set the initial state - such as interface orientation - required for your tests before they run. The setUp method is a good place to do this.
+    }
+
+    override func tearDown() {
+        // Put teardown code here. This method is called after the invocation of each test method in the class.
+    }
+
+    func testExample() {
+        // Use recording to get started writing UI tests.
+        // Use XCTAssert and related functions to verify your tests produce the correct results.
+    }


I guess this default code can be removed?

I honestly don't remember. Probably, I think I just added the tests for the two platforms, but I didn't really look at the code.

MaxHasADHD · 2020-03-31T12:08:19Z

TraktKitTvOSTests/TraktKitTvOSTests.swift

+    override func setUp() {
+        // Put setup code here. This method is called before the invocation of each test method in the class.
+    }
+
+    override func tearDown() {
+        // Put teardown code here. This method is called after the invocation of each test method in the class.
+    }
+
+    func testExample() {
+        // This is an example of a functional test case.
+        // Use XCTAssert and related functions to verify your tests produce the correct results.
+    }
+
+    func testPerformanceExample() {
+        // This is an example of a performance test case.
+        self.measure {
+            // Put the code you want to measure the time of here.
+        }
+    }


MaxHasADHD · 2020-03-31T12:09:16Z

Sorry for the wait on this. I became pretty burnt out. I'll try to be more active when I can.

Michelasso · 2020-03-31T16:59:19Z

Yeah, I am pretty busy myself, now. Working online from home. Also I didn't touch the project for quite a long time. IB for tvOS went banana with Xcode 11.

So how does it work? Should I apply the changes requested and update?

MaxHasADHD · 2020-11-09T12:11:37Z

@Michelasso if you can, yes. Resolve the conflicts, and address comments. No rush though

Ported TraktKit to tvOS

c769609

Converted frameworks to Universal frameworks Added test for tvOS and macOS

Michelasso commented Jul 1, 2019

View reviewed changes

Fixed wrong URL formations - Cleaned up comments - macOS tests on Tra…

aef0b0f

…ktKitMacOS (removed TraktKitMacOS-tests)

MaxHasADHD requested changes Mar 31, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ported TraktKit to tvOS #36

Ported TraktKit to tvOS #36

Michelasso commented Jun 29, 2019

Michelasso commented Jun 30, 2019

Michelasso Jul 1, 2019

MaxHasADHD Mar 31, 2020

Michelasso commented Jul 1, 2019

MaxHasADHD Mar 31, 2020

MaxHasADHD Mar 31, 2020

Michelasso Mar 31, 2020

MaxHasADHD Mar 31, 2020

Michelasso Mar 31, 2020

MaxHasADHD Mar 31, 2020

MaxHasADHD Mar 31, 2020

MaxHasADHD Mar 31, 2020

MaxHasADHD Mar 31, 2020

MaxHasADHD Mar 31, 2020

MaxHasADHD Mar 31, 2020

Michelasso Mar 31, 2020 •

edited

Loading

MaxHasADHD Mar 31, 2020

MaxHasADHD commented Mar 31, 2020

Michelasso commented Mar 31, 2020

MaxHasADHD commented Nov 9, 2020

Ported TraktKit to tvOS #36

Are you sure you want to change the base?

Ported TraktKit to tvOS #36

Conversation

Michelasso commented Jun 29, 2019

Michelasso commented Jun 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Michelasso commented Jul 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Michelasso Mar 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxHasADHD commented Mar 31, 2020

Michelasso commented Mar 31, 2020

MaxHasADHD commented Nov 9, 2020

Michelasso Mar 31, 2020 •

edited

Loading